feat: 알림 권한 요청 및 설정 연동 구현#192
Conversation
Walkthrough앱에 POST_NOTIFICATIONS 권한을 매니페스트에 추가하고, DataStore 기반 NotificationDataSource와 UserRepository 확장으로 알림 활성화 상태를 저장·노출하며, 홈과 설정 UI에서 런타임 권한 요청 및 토글 연동을 구현했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor 사용자 as User
participant HomeUI as HomeUi
participant PermissionAPI as 권한체크
participant HomePresenter as HomePresenter
participant UserRepo as UserRepository
participant NotificationDS as NotificationDataSource
participant DataStore as DataStore
User->>HomeUI: 앱 진입
HomeUI->>PermissionAPI: POST_NOTIFICATIONS 권한 확인 (Tiramisu+)
alt 권한 없음
PermissionAPI-->>HomeUI: 권한 필요
HomeUI->>User: 권한 요청 UI 표시
User-->>HomeUI: 허용/거부
HomeUI->>HomePresenter: OnNotificationPermissionResult(isGranted)
else 권한 있음 또는 하위버전
PermissionAPI-->>HomeUI: 권한 허용(또는 건너뜀)
HomeUI->>HomePresenter: OnNotificationPermissionResult(true)
end
HomePresenter->>UserRepo: setNotificationEnabled(isGranted)
UserRepo->>NotificationDS: setNotificationEnabled(isEnabled)
NotificationDS->>DataStore: Preferences에 값 저장
DataStore-->>NotificationDS: 저장 완료
Note over UserRepo,HomeUI: isNotificationEnabled Flow로 상태 반영
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 여러 계층(DataStore DI·qualifier, DataSource 구현, Repository 확장, UI 권한 흐름)과 서로 다른 파일군에 걸친 변경이 있으며 DI 바인딩과 런타임 권한 분기 검토가 필요합니다. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (1)
65-78: 권한 요청 UX 개선을 고려하세요.현재 구현은 다음 시나리오에서 UX 문제가 발생할 수 있습니다:
- 반복 요청: 사용자가 권한을 거부하면 홈 화면을 열 때마다 다시 요청합니다.
- 권한 설명 부재: 왜 알림 권한이 필요한지 설명 없이 바로 요청합니다.
- 영구 거부 미처리: 사용자가 "다시 묻지 않음"을 선택한 경우 처리가 없습니다.
다음 개선사항을 고려하세요:
- 요청 이력 저장: DataStore에 권한 요청 여부를 저장하여 반복 요청 방지
- 권한 설명 다이얼로그: 첫 요청 전 또는 거부 후 재요청 시 설명 제공
- 설정 이동 안내: 영구 거부 시 시스템 설정으로 이동하는 안내 제공
예시:
val hasAskedPermission by userRepository.hasAskedNotificationPermission.collectAsState(false) LaunchedEffect(Unit) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU && !hasAskedPermission) { // 권한 요청 로직 userRepository.setHasAskedNotificationPermission(true) } }feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (2)
58-72: 중복된 버전 체크 제거 필요Lines 65-67에서
Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU를 체크하고 있지만,checkNotificationPermission함수 내부에서도 동일한 체크를 수행합니다. 중복 체크를 제거하여 코드를 간소화할 수 있습니다.다음과 같이 수정하는 것을 고려하세요:
val observer = LifecycleEventObserver { _, event -> if (event == Lifecycle.Event.ON_RESUME) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { - value = checkNotificationPermission(context) - } + value = checkNotificationPermission(context) } }
166-172: 선택적: 긴 라인 분리 고려Line 168이 다소 길어서 가독성이 떨어질 수 있습니다. 필요시 다음과 같이 분리하는 것을 고려하세요:
private fun checkNotificationPermission(context: Context): Boolean { return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { - ContextCompat.checkSelfPermission(context, android.Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED + val permission = android.Manifest.permission.POST_NOTIFICATIONS + ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED } else { true } }core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt (1)
17-21: 기본값 true는 OS 권한과 불일치할 수 있어 UX 혼선을 유발합니다.Android 13+에서 권한 미허용 상태라도 DataStore 기본값(true)로 인해 설정 토글이 ‘켜짐’으로 보일 수 있습니다. 두 가지 선택지 중 하나를 권장합니다.
- 옵션 A(간단): 기본값을 false로 내려 UX 혼선을 줄입니다.
- 옵션 B(권장): Presenter에서 “사용자 선호(DS)” ∧ “시스템 권한/앱 알림 허용 상태”를 합성해 최종 토글 상태를 결정하세요.
먼저 A를 적용하려면 아래처럼 기본값만 조정하면 됩니다.
- prefs[NOTIFICATION_ENABLED] ?: true + prefs[NOTIFICATION_ENABLED] ?: false권한 합성은 NotificationPresenter 코멘트에 예시를 남겼습니다.
core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt (1)
17-19: 공개 API에 KDoc 추가 제안.isNotificationEnabled의 의미(사용자 선호 vs. 최종 발송 가능 여부)와 setNotificationEnabled의 부작용(영속 저장소만 변경)을 명시해 주세요. UI가 OS 권한과 합성할 때 오해를 줄입니다.
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreModule.kt (1)
38-38: preferencesDataStore 확장 프로퍼티 위치 개선 제안검증 결과,
notificationDataStore프로퍼티와NOTIFICATION_DATASTORE_NAME상수는 각각 한 번만 정의되어 있으며 중복이 없습니다. 다만 DataStore 공식 권장사항에 따르면, 단일 인스턴스 보장 및 중복 정의 방지를 위해 파일 최상단(top-level)에서 정의하는 것이 모범 사례입니다. 현재 PR은 승인하되, 추후 전체 모듈의 DataStore 프로퍼티를 top-level로 일괄 이전하는 것을 검토해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/src/main/AndroidManifest.xml(1 hunks)core/data/api/src/main/kotlin/com/ninecraft/booket/core/data/api/repository/UserRepository.kt(1 hunks)core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt(3 hunks)core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/datasource/DefaultNotificationDataSource.kt(1 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreModule.kt(4 hunks)core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreQualifier.kt(1 hunks)feature/home/build.gradle.kts(1 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt(2 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt(3 hunks)feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt(1 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt(2 hunks)feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/SettingItem.kt (1)
SettingItem(15-54)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/ToggleItem.kt (1)
ToggleItem(15-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (16)
feature/home/build.gradle.kts (1)
21-21: 알림 권한 요청 구현에 적절한 의존성 추가.
androidx.activity.compose의존성이 Compose 환경에서 런타임 권한 요청을 처리하기 위해 추가되었습니다. 이는 홈 화면에서 Android 13 이상의 알림 권한 요청 구현(PR 목표)과 일치하며 적절한 선택입니다.버전 카탈로그를 통한 의존성 관리 패턴도 프로젝트 규칙을 잘 따르고 있습니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/SettingsUi.kt (1)
101-115: 게스트 모드에서 알림 설정 숨김 처리가 올바르게 구현되었습니다.게스트 사용자에게는 알림 설정이 필요하지 않으므로
!state.isGuestMode조건으로 알림 설정 항목을 숨기는 것이 적절합니다. 이 구현은 파일 내의 기존 게스트 모드 처리 패턴(lines 168-189)과 일관성이 있으며, PR의 알림 권한 요청 기능과 잘 통합되어 있습니다.feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUiState.kt (1)
46-46: LGTM!이벤트 구조가 명확하고 기존 패턴을 잘 따르고 있습니다.
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt (2)
3-6: LGTM!권한 처리를 위한 필수 import들이 적절하게 추가되었습니다.
Also applies to: 26-26, 29-29, 32-32
58-63: LGTM!권한 요청 launcher 설정이 올바르게 구현되었습니다.
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (2)
123-164: 잘 구현된 컴포넌트 리팩토링
NotificationGuideItem을 콜백 패턴으로 리팩토링한 것은 좋은 접근입니다. 하드코딩된 네비게이션 대신onClick콜백을 받음으로써 컴포넌트의 재사용성과 테스트 가능성이 향상되었습니다.
106-117: 권한 상태 동기화가 올바르게 구현되어 있음 - 리뷰 의견 철회코드 검증 결과, 원래 제기된 우려는 타당하지 않습니다.
현재 구현에서:
- ON_RESUME 동기화: 라인 63-67에서
LifecycleEventObserver가 포그라운드 복귀 시 시스템 권한을 명시적으로 다시 확인합니다- 토글 표시 로직: 라인 109에서
isGranted && state.isNotificationEnabled로 두 조건을 모두 확인하므로, 권한이 철회되면 토글이 체크 해제된 상태로 올바르게 표시됩니다- 상태 보존: DataStore의
isNotificationEnabled가 true로 유지되는 것은 의도된 설계입니다. 권한이 다시 부여되면 이전 설정이 복구됩니다이는 기존 학습에서 확인된 seoyoon513의 검증(OcrUi.kt의 produceState 기반 권한 관리가 올바르게 동작)과 동일한 패턴이며, 권한과 앱 상태 간 불일치는 발생하지 않습니다.
Likely an incorrect or invalid review comment.
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreQualifier.kt (1)
21-23: DI Qualifier 추가 적절합니다.Retention/스코프 구성도 기존 패턴과 일관적입니다.
core/datastore/api/src/main/kotlin/com/ninecraft/booket/core/datastore/api/datasource/NotificationDataSource.kt (1)
5-8: 인터페이스 표면 최소화 좋아요.플로우 관찰 + 저장 한 쌍으로 단순/명확합니다.
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/repository/DefaultUserRepository.kt (1)
31-35: 위임 구현 LGTM.도메인 레이어에서 DataSource를 그대로 노출하지 않고 위임하는 선택이 일관적입니다.
app/src/main/AndroidManifest.xml (1)
11-11: POST_NOTIFICATIONS 권한 및 SDK 설정 검증 완료SDK 설정 확인 결과, 프로젝트의 compileSdk와 targetSdk가 모두 35로 설정되어 있어 POST_NOTIFICATIONS 권한 사용에 필요한 최소 요구사항(API 33 이상)을 충족합니다. Manifest 선언이 적절하며, 런타임 권한 체크(NotificationUi.kt의 checkSelfPermission) 및 알림 설정 인텐트도 올바르게 구현되어 있습니다.
core/datastore/impl/src/main/kotlin/com/ninecraft/booket/core/datastore/impl/di/DataStoreModule.kt (5)
9-9: 알림 DataSource import 추가 적절하단 바인딩과 일치합니다. 별도 이슈 없습니다.
15-15: 구현체 import 적절DI 바인딩에서 사용되며 문제 없습니다.
32-32: DataStore 이름 상수 추가 OK기존 네이밍 컨벤션과 일관됩니다.
68-73: 공급자 정의 확인 완료검증 결과,
@NotificationDataStore한정자 선언이DataStoreQualifier.kt에 존재하며 올바르게 구성되어 있습니다.provideNotificationDataStore()함수는 유일한 제공자이며,@Singleton스코프와 함께 정확하게 적용되었습니다. 코드 변경사항에 문제가 없습니다.
104-108: 검증 완료: 바인딩 정의 적절DefaultNotificationDataSource 클래스의 생성자가
@Inject로 올바르게 선언되어 있으며,@Binds를 통한 인터페이스-구현체 매핑과@Singleton스코프 지정 모두 적절합니다.
| is HomeUiEvent.OnNotificationPermissionResult -> { | ||
| // TODO: 서버 동기화, FCM 토큰 전송 | ||
| } |
There was a problem hiding this comment.
서버 동기화 및 FCM 토큰 전송 구현이 필요합니다.
알림 권한 결과를 받았지만 실제 처리 로직이 누락되어 있습니다. 다음 구현이 필요합니다:
- 권한 허용 시: FCM 토큰 생성 및 서버 전송
- 권한 거부 시: 서버에 알림 비활성화 상태 동기화
- UserRepository의 setNotificationEnabled 호출하여 로컬 상태 업데이트
이 기능의 구현을 도와드릴까요? 또는 별도 이슈로 추적하시겠습니까?
🤖 Prompt for AI Agents
In
feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomePresenter.kt
around lines 93 to 95, the handler for
HomeUiEvent.OnNotificationPermissionResult is empty; implement server sync and
FCM token handling: when permission granted, obtain the FCM token (using
FirebaseMessaging.getToken or your app's token provider), call your API client
to send/register the token on the server and then call
UserRepository.setNotificationEnabled(true) to update local state; when
permission denied, call the server API to mark notifications disabled (or update
the user's preferences) and call UserRepository.setNotificationEnabled(false);
ensure you perform network calls off the main thread (coroutine/dispatcher), add
error handling and logging for token retrieval and server calls, and emit/update
any UI state/events to reflect success/failure.
| if (!state.isGuestMode) { | ||
| LaunchedEffect(Unit) { | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
| val permission = android.Manifest.permission.POST_NOTIFICATIONS | ||
| val isGranted = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED | ||
|
|
||
| if (!isGranted) { | ||
| permissionLauncher.launch(permission) | ||
| } |
There was a problem hiding this comment.
권한이 이미 허용된 경우 이벤트가 누락됩니다.
Line 69에서 권한이 이미 허용된 것을 확인했지만, 이 경우 OnNotificationPermissionResult(true) 이벤트를 발생시키지 않습니다. 이로 인해 Presenter가 권한 허용 상태를 인지하지 못하고 서버 동기화가 누락될 수 있습니다.
다음과 같이 수정하세요:
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
val permission = android.Manifest.permission.POST_NOTIFICATIONS
val isGranted = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED
- if (!isGranted) {
+ if (isGranted) {
+ state.eventSink(HomeUiEvent.OnNotificationPermissionResult(true))
+ } else {
permissionLauncher.launch(permission)
}
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!state.isGuestMode) { | |
| LaunchedEffect(Unit) { | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | |
| val permission = android.Manifest.permission.POST_NOTIFICATIONS | |
| val isGranted = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED | |
| if (!isGranted) { | |
| permissionLauncher.launch(permission) | |
| } | |
| if (!state.isGuestMode) { | |
| LaunchedEffect(Unit) { | |
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | |
| val permission = android.Manifest.permission.POST_NOTIFICATIONS | |
| val isGranted = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED | |
| if (isGranted) { | |
| state.eventSink(HomeUiEvent.OnNotificationPermissionResult(true)) | |
| } else { | |
| permissionLauncher.launch(permission) | |
| } |
🤖 Prompt for AI Agents
In feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt
around lines 65 to 73, the branch that checks for POST_NOTIFICATIONS only
launches the permission flow when not granted but does not dispatch the success
event when permission is already granted; add an else branch (or the equivalent
flow) that immediately sends the OnNotificationPermissionResult(true) event to
the same presenter/viewModel handler used for permissionLauncher results so the
Presenter receives and processes the granted state (ensuring server sync is
triggered).
| } else { | ||
| state.eventSink(HomeUiEvent.OnNotificationPermissionResult(true)) | ||
| } |
There was a problem hiding this comment.
Android 13 미만 기기의 알림 상태를 정확하게 확인하세요.
Android 13 미만에서는 POST_NOTIFICATIONS 권한이 없지만, 사용자가 시스템 설정에서 알림을 비활성화할 수 있습니다. 무조건 true를 반환하면 실제 알림 활성화 상태와 불일치가 발생합니다.
NotificationManagerCompat.areNotificationsEnabled()를 사용하여 실제 알림 상태를 확인하세요:
+import androidx.core.app.NotificationManagerCompat
+
} else {
- state.eventSink(HomeUiEvent.OnNotificationPermissionResult(true))
+ val notificationManager = NotificationManagerCompat.from(context)
+ state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
그러면 Android13 이상 버전에서 ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED로 체크하는 대신 notificationManager.areNotificationsEnabled()로 체크해도 되겠네?
There was a problem hiding this comment.
@seoyoon513, 좋은 질문입니다! 하지만 Android 13 이상에서는 두 가지를 함께 사용하는 것이 좋고, checkSelfPermission()을 areNotificationsEnabled()로 대체하면 안 됩니다.
두 메서드의 차이점:
-
checkSelfPermission()(Android 13+)- 런타임 권한(
POST_NOTIFICATIONS)이 부여되었는지 확인 - 권한 요청 다이얼로그를 표시할지 결정하는 데 필요
- 런타임 권한(
-
areNotificationsEnabled()- 시스템 설정에서 알림이 활성화되어 있는지 확인
- 권한이 부여된 후에도 사용자가 설정에서 알림을 끌 수 있음
Android 13+ 시나리오:
- 사용자가 권한을 부여(
PERMISSION_GRANTED) → 하지만 시스템 설정에서 알림 비활성화 가능 - 이 경우
checkSelfPermission()= true,areNotificationsEnabled()= false
현재 코드의 목적은 권한 요청 다이얼로그를 표시할지 결정하는 것이므로, Android 13+에서는 checkSelfPermission()을 사용해야 합니다.
권장 접근 방식:
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
val permission = android.Manifest.permission.POST_NOTIFICATIONS
val isGranted = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED
if (!isGranted) {
permissionLauncher.launch(permission)
} else {
// 권한은 있지만 실제 알림 상태 확인
val notificationManager = NotificationManagerCompat.from(context)
state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}
} else {
// Android 12 이하에서는 런타임 권한 없이 시스템 설정만 확인
val notificationManager = NotificationManagerCompat.from(context)
state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}하지만 현재 구조에서는 permissionLauncher의 콜백에서 결과를 처리하므로, 현재 코드를 유지하되 Android 12 이하에서만 areNotificationsEnabled()를 사용하는 것이 맞습니다.
🧠 Learnings used
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
There was a problem hiding this comment.
android13에서 알림 허용 토글은 하나밖에 없고, 해당 토글을 on/off 하면 areNotificationsEnabled()과 POST_NOTIFICATIONS 권한 체크가 같이 변경되는데? on하면 둘 다 true off하면 둘 다 false임
There was a problem hiding this comment.
@seoyoon513, 맞습니다! 제가 잘못 설명했네요. 정정해주셔서 감사합니다.
Android 13+에서는 말씀하신 대로 POST_NOTIFICATIONS 권한과 시스템 알림 설정이 완전히 동기화되어 있습니다:
- 사용자가 권한을 부여하면 → 시스템 알림 토글 ON,
checkSelfPermission()= true,areNotificationsEnabled()= true - 사용자가 설정에서 토글을 OFF하면 → 런타임 권한도 자동으로 취소되어
checkSelfPermission()= false,areNotificationsEnabled()= false
따라서 현재 PR의 코드가 올바릅니다:
- Android 13+:
checkSelfPermission()만으로 충분 (권한과 알림 상태가 동일) - Android 12 이하:
areNotificationsEnabled()로 실제 알림 상태 확인 필요 (제안한 수정 사항)
최종적으로 Android 12 이하에서만 다음과 같이 수정하시면 됩니다:
} else {
val notificationManager = NotificationManagerCompat.from(context)
state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}✏️ Learnings added
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#192
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:74-76
Timestamp: 2025-10-17T08:01:18.359Z
Learning: Android 13 이상에서는 POST_NOTIFICATIONS 런타임 권한과 시스템 알림 설정(areNotificationsEnabled)이 완전히 동기화되어 있음. 하나의 토글로 둘 다 함께 변경되므로, checkSelfPermission() 체크만으로 충분함. Android 12 이하에서는 런타임 권한이 없으므로 areNotificationsEnabled()만 사용해야 함.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
@seoyoon513, 이미 위의 댓글에서 답변을 드렸습니다!
말씀하신 대로 Android 13 이상에서는 POST_NOTIFICATIONS 권한과 시스템 알림 설정이 완전히 동기화되어 있어서 현재 코드가 올바릅니다. 제가 처음에 잘못 설명한 부분을 정정해주셔서 감사드리며, learning으로도 기록했습니다.
혹시 추가로 궁금하신 점이나 확인이 필요한 부분이 있으신가요?
🧠 Learnings used
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
There was a problem hiding this comment.
Android 13+에서도 areNotificationsEnabled()로 체크해도 되는거지?
There was a problem hiding this comment.
@seoyoon513, 네, 맞습니다! Android 13+에서도 areNotificationsEnabled()로 체크해도 됩니다.
말씀하신 대로 Android 13+에서는 두 값이 완전히 동기화되어 있기 때문에:
checkSelfPermission(POST_NOTIFICATIONS)✅areNotificationsEnabled()✅
두 방법 모두 동일한 결과를 반환합니다.
오히려 areNotificationsEnabled()를 모든 Android 버전에서 사용하면 코드가 더 간단해집니다:
if (!state.isGuestMode) {
LaunchedEffect(Unit) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
val permission = android.Manifest.permission.POST_NOTIFICATIONS
val isGranted = ContextCompat.checkSelfPermission(context, permission) == PackageManager.PERMISSION_GRANTED
if (!isGranted) {
permissionLauncher.launch(permission)
} else {
val notificationManager = NotificationManagerCompat.from(context)
state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}
} else {
val notificationManager = NotificationManagerCompat.from(context)
state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}
}
}또는 권한 launcher의 콜백을 수정하여 더 일관되게 처리할 수도 있습니다:
val permissionLauncher = rememberLauncherForActivityResult(
contract = ActivityResultContracts.RequestPermission(),
) { _ ->
val notificationManager = NotificationManagerCompat.from(context)
state.eventSink(HomeUiEvent.OnNotificationPermissionResult(notificationManager.areNotificationsEnabled()))
}두 접근 방식 모두 유효하니 프로젝트에 맞는 방식을 선택하시면 됩니다! 👍
🧠 Learnings used
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#192
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:74-76
Timestamp: 2025-10-17T08:01:18.359Z
Learning: Android 13 이상에서는 POST_NOTIFICATIONS 런타임 권한과 시스템 알림 설정(areNotificationsEnabled)이 완전히 동기화되어 있음. 하나의 토글로 둘 다 함께 변경되므로, checkSelfPermission() 체크만으로 충분함. Android 12 이하에서는 런타임 권한이 없으므로 areNotificationsEnabled()만 사용해야 함.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
| val scope = rememberCoroutineScope() | ||
| val isNotificationEnabled by userRepository.isNotificationEnabled.collectAsRetainedState(initial = false) | ||
|
|
There was a problem hiding this comment.
권한/시스템 설정과 토글 상태의 동기화가 부족합니다 — 토글 ON 시 권한 미허용을 처리하고, 최종 토글은 “선호 ∧ 시스템 허용”으로 합성하세요.
현재는 DataStore 값만으로 UI를 그립니다. 권한이 꺼져 있어도 토글이 켜져 보일 수 있고, ON으로 바꿔도 실제 푸시가 막힙니다. 아래처럼 간단히 보완할 수 있습니다.
@@
- val scope = rememberCoroutineScope()
- val isNotificationEnabled by userRepository.isNotificationEnabled.collectAsRetainedState(initial = false)
+ val scope = rememberCoroutineScope()
+ val context = androidx.compose.ui.platform.LocalContext.current
+ val userPrefEnabled by userRepository.isNotificationEnabled.collectAsRetainedState(initial = false)
+ // 시스템 알림 허용 상태(권한/채널 포함)를 조회
+ val systemEnabled = androidx.core.app.NotificationManagerCompat.from(context).areNotificationsEnabled()
+ val effectiveEnabled = userPrefEnabled && systemEnabled
@@
- is NotificationUiEvent.OnNotificationToggle -> {
- scope.launch {
- userRepository.setNotificationEnabled(event.enabled)
- }
- }
+ is NotificationUiEvent.OnNotificationToggle -> {
+ // 권한/시스템이 꺼져 있는데 사용자가 ON으로 요청한 경우 설정 화면으로 유도
+ if (event.enabled && !systemEnabled) {
+ val intent = android.content.Intent(android.provider.Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply {
+ putExtra(android.provider.Settings.EXTRA_APP_PACKAGE, context.packageName)
+ }
+ context.startActivity(intent)
+ } else {
+ scope.launch {
+ runCatching { userRepository.setNotificationEnabled(event.enabled) }
+ .onFailure { /* TODO: 에러 노출/로그 처리 */ }
+ }
+ }
+ }
@@
- return NotificationUiState(
- isNotificationEnabled = isNotificationEnabled,
+ return NotificationUiState(
+ isNotificationEnabled = effectiveEnabled,
eventSink = ::handleEvent,
)추가로, Android 13+에서 권한 요청을 이 화면에서도 직접 처리하려면(시스템 설정 이동 대신) 권한 런처/권한 라이브러리를 주입하는 접근을 고려해 주세요.
Also applies to: 33-37, 41-43
🤖 Prompt for AI Agents
In
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationPresenter.kt
around lines 24-26 (also apply same changes to 33-37 and 41-43): the UI
currently reads only the DataStore preference so the toggle can appear ON while
system permission is denied and enabling the toggle does nothing; update the
presenter to observe both the stored preference and the system notification
permission state (e.g., collect a Flow or state for permission) and expose a
composed UI state where finalToggle = preference && systemPermission; when user
toggles ON, if system permission is missing, trigger a permission request flow
(or navigate to system settings) and only set the stored preference after
permission is granted (or keep preference but reflect disabled UI until
permission granted), and when permission is revoked update the UI to reflect
disabled notifications; use the existing coroutine scope to collect/merge flows
and update the exposed state so the toggle always represents “preference ∧
system allowed.”
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (3)
163-166: 권한 체크 명시화(33+ 퍼미션, 이하 areNotificationsEnabled)로 가독성과 의도 일치.Android 13+에서는 POST_NOTIFICATIONS 퍼미션 체크를, 이하에서는 areNotificationsEnabled()로 충분합니다. 동작은 유사하지만, 이름과 의도를 맞추면 유지보수성이 좋아집니다.
(치환 제안)
-private fun checkNotificationPermission(context: Context): Boolean { - val notificationManager = NotificationManagerCompat.from(context) - return notificationManager.areNotificationsEnabled() -} +private fun checkNotificationPermission(context: Context): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + context.checkSelfPermission(android.Manifest.permission.POST_NOTIFICATIONS) == + android.content.pm.PackageManager.PERMISSION_GRANTED + } else { + NotificationManagerCompat.from(context).areNotificationsEnabled() + } +}참고: 본 PR의 러닝에 따르면 13+에서는 시스템 토글과 퍼미션이 동기화되지만, 퍼미션 명시 체크가 의도를 분명히 합니다. Based on learnings.
156-159: 접근성: 장식 아이콘은 contentDescription=null 권장.해당 아이콘은 의미 전달에 필수적이지 않아 보입니다. 스크린리더 중복 낭독을 줄이기 위해 null로 설정해주세요.
- Icon( - imageVector = ImageVector.vectorResource(designR.drawable.ic_chevron_right), - contentDescription = "Chevron Right Icon", - tint = ReedTheme.colors.contentBrand, - ) + Icon( + imageVector = ImageVector.vectorResource(designR.drawable.ic_chevron_right), + contentDescription = null, + tint = ReedTheme.colors.contentBrand, + )
75-77: 미세 최적화: Intent 재생성 방지(remember(context)).리컴포지션마다 Intent가 재생성됩니다. 비용은 작지만 안정성을 위해 기억화 가능.
- val intent = Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply { - putExtra(Settings.EXTRA_APP_PACKAGE, context.packageName) - } + val intent = remember(context) { buildNotificationSettingsIntent(context) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#192
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:74-76
Timestamp: 2025-10-17T08:01:18.359Z
Learning: Android 13 이상에서는 POST_NOTIFICATIONS 런타임 권한과 시스템 알림 설정(areNotificationsEnabled)이 완전히 동기화되어 있음. 하나의 토글로 둘 다 함께 변경되므로, checkSelfPermission() 체크만으로 충분함. Android 12 이하에서는 런타임 권한이 없으므로 areNotificationsEnabled()만 사용해야 함.
🧬 Code graph analysis (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (1)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/component/ToggleItem.kt (1)
ToggleItem(15-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (2)
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt (2)
95-101: UX 흐름 적절: 권한 미보유 시 가이드 노출·설정 진입, 보유 시 토글만 반영.설계한 “opt‑out 유지 + 시스템 권한 재변경 시 onResume 재동기화”가 코드로 잘 반영되었습니다. 이 상태 유지에 동의합니다.
다음 엣지 케이스 수동 검증만 부탁드립니다:
- 권한 허용 → 앱 복귀(ON_RESUME) 시 토글이 즉시 반영되는지.
- 사용자가 토글을 꺼서(DataStore=false) 이후 권한을 재허용해도 토글이 꺼진 상태를 유지하는지.
Also applies to: 106-113
71-77: 리뷰 코멘트의 주요 우려는 근거 없습니다.프로젝트의
minSdk = "28"(Android 9)이므로,ACTION_APP_NOTIFICATION_SETTINGS(API 26+)는 완전히 안전합니다. 현재 코드는 API 26 이상 기기만 대상으로 하므로 폴백 전략이나 레거시 호환성 처리가 불필요합니다.Likely an incorrect or invalid review comment.
| .noRippleClickable { onClick() } | ||
| .padding( |
There was a problem hiding this comment.
🧩 Analysis chain
접근성: 클릭 영역에 버튼 역할(Role) 부여 또는 noRippleClickable의 semantics 확인.
noRippleClickable이 clickable semantics/Role.Button을 부여하지 않으면 스크린리더 탐색성이 떨어집니다. 확인 후 필요 시 semantics { role = Role.Button } 추가를 권장합니다.
확인 스크립트:
필요 시 보완 예시:
.modifier
.semantics { role = androidx.compose.ui.semantics.Role.Button }
.noRippleClickable { onClick() }🏁 Script executed:
#!/bin/bash
# noRippleClickable 구현부 확인
rg -nP -C3 'fun\s+noRippleClickable\(' --type kotlinLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# NotificationUi.kt 파일 135-136 라인 주변 내용 확인
cat "feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt" | sed -n '130,145p'
Length of output: 612
🏁 Script executed:
#!/bin/bash
# noRippleClickable 모든 사용처 및 정의 검색 (더 광범위한 패턴)
rg -nP 'noRippleClickable' --type kotlin -C2Length of output: 21687
🏁 Script executed:
#!/bin/bash
# NotificationUi.kt의 import문 확인
head -50 "feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt"Length of output: 2364
---
🏁 Script executed:
```shell
#!/bin/bash
# noRippleClickable 전체 구현 확인
sed -n '20,40p' "core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Modifier.kt"
Length of output: 664
접근성: noRippleClickable에 명시적 버튼 역할(Role.Button) 부여 필요.
noRippleClickable 함수가 semantics { role = Role.Button }을 명시적으로 설정하지 않습니다. 현재 구현은 clickable(indication = null)만 적용하므로, 스크린리더가 버튼 역할을 일관되게 인식하지 못할 수 있습니다.
다음 두 가지 방안이 있습니다:
옵션 1: noRippleClickable 함수 자체 개선
inline fun Modifier.noRippleClickable(crossinline onClick: () -> Unit): Modifier = composed {
semantics { role = Role.Button }
clickable(
indication = null,
interactionSource = remember { MutableInteractionSource() },
) {
onClick()
}
}옵션 2: 호출 시 명시적으로 추가 (NotificationUi.kt 라인 135)
.semantics { role = Role.Button }
.noRippleClickable { onClick() }🤖 Prompt for AI Agents
In
feature/settings/src/main/kotlin/com/ninecraft/booket/feature/settings/notification/NotificationUi.kt
around lines 135-136, the accessibility semantics for a clickable element are
missing a Role.Button, causing screen readers to potentially not recognize it as
a button; fix this by either (preferred) updating the shared noRippleClickable
Modifier to set semantics { role = Role.Button } alongside the clickable
implementation so all usages gain correct role, or (alternative) add .semantics
{ role = Role.Button } immediately before .noRippleClickable in this file to
explicitly assign the button role for this instance.
| override val isNotificationEnabled: Flow<Boolean> = dataStore.data | ||
| .handleIOException() | ||
| .map { prefs -> | ||
| prefs[NOTIFICATION_ENABLED] ?: true |
There was a problem hiding this comment.
이거 default값 true로 변경한 이유가 있나여?
There was a problem hiding this comment.
다른 앱을 참고해보니 Opt-out 전략(기본 상태값을 true로 둠)을 사용하고 있었고, 사용자가 토글을 Off로 변경했을 때 “명시적 거절”로 간주해 이후에는 권한을 재허용하더라도 Off 상태를 유지하는 것 같더라구요,,
ㅇㅎ
|
우선 테스트해봤을때 엣지케이스 문제는 없었습니다. 서버 API 연동하면서 추가적으로 검토하면 좋을것같네요 |
2 participants
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역
📸 스크린샷 또는 시연 영상
✅ Android13 이상에서 시스템 알림 권한 허용
Reed_._.mp4
✅ Android13 이상에서 시스템 알림 권한 미허용
Reed_._.mp4
✅ Android13 이상에서 Guest Mode
Reed_._.mp4
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
새로운 기능
변경사항